nxos_lacp: updated tests to handle platforms not supporting lacp system mac command #64074
Conversation
ansibot
added
affects_2.10
justjais
removed
the
needs_triage
| @@ -1,11 +1,18 @@ | |||
| --- | |||
|
|
|||
There was a problem hiding this comment.
Let's also add a few more tests on platforms that support both properties to be complete.
- Configure both properties (replace them both with new values)
- Configure mac only (playbook specifies no values so mac gets removed)
| @@ -6,21 +6,38 @@ | |||
| nxos_feature: | |||
| feature: lacp | |||
|
|
|||
| - set_fact: | |||
There was a problem hiding this comment.
I see the following failure running the merged test from your nxos_lacp PR on the following release images (camden, dplus, evergreen and freeport)
These older release images likey do not have support for this command.
Fails in:
camden (I2)
dplus (I4)
evergreen (I5)
freeport (I6)
I suggest you do the following:
- Check to see if priority works on these older releases and still test that but omit mac.
- If neither property is supported, you can skip the tests for tag (I2 – I6)
TASK [nxos_lacp : Merged] ************************************************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_lacp/tests/cli/merged.yaml:16
<camden-n9k.example.com.cisco.com> attempting to start connection
<camden-n9k.example.com.cisco.com> using connection plugin network_cli
Found ansible-connection at path /root/agents-ci/ansible/bin/ansible-connection
<camden-n9k.example.com.cisco.com> found existing local domain socket, using it!
<camden-n9k.example.com.cisco.com> updating play_context for connection
<camden-n9k.example.com.cisco.com>
<camden-n9k.example.com.cisco.com> local domain socket path is /root/.ansible/pc/0355643899
<camden-n9k.example.com.cisco.com> ESTABLISH LOCAL CONNECTION FOR USER: root
<camden-n9k.example.com.cisco.com> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944 `" && echo ansible-tmp-1572449641.13-1032907249944="` echo /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944 `" ) && sleep 0'
Using module file /root/agents-ci/ansible/lib/ansible/modules/network/nxos/nxos_lacp.py
<camden-n9k.example.com.cisco.com> PUT /root/.ansible/tmp/ansible-local-10715130OGa/tmpRge0Xc TO /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py
<camden-n9k.example.com.cisco.com> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/ /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py && sleep 0'
<camden-n9k.example.com.cisco.com> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py && sleep 0'
<camden-n9k.example.com.cisco.com> EXEC /bin/sh -c 'rm -f -r /root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/ > /dev/null 2>&1 && sleep 0'
The full traceback is:
Traceback (most recent call last):
File "/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py", line 102, in <module>
_ansiballz_main()
File "/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py", line 94, in _ansiballz_main
invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)
File "/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py", line 40, in invoke_module
runpy.run_module(mod_name='ansible.modules.network.nxos.nxos_lacp', init_globals=None, run_name='__main__', alter_sys=False)
File "/usr/lib/python2.7/runpy.py", line 192, in run_module
fname, loader, pkg_name)
File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
exec code in run_globals
File "/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/modules/network/nxos/nxos_lacp.py", line 188, in <module>
File "/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/modules/network/nxos/nxos_lacp.py", line 183, in main
File "/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/module_utils/network/nxos/config/lacp/lacp.py", line 69, in execute_module
File "/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/module_utils/connection.py", line 187, in __rpc__
ansible.module_utils.connection.ConnectionError: lacp system-mac 00c1.4c00.bd15 role primary
^
% Invalid command at '^' marker.
camden-n9k.example.com(config)#
fatal: [camden-n9k.example.com.cisco.com]: FAILED! => {
"changed": false,
"module_stderr": "Traceback (most recent call last):\n File \"/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py\", line 102, in <module>\n _ansiballz_main()\n File \"/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py\", line 94, in _ansiballz_main\n invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n File \"/root/.ansible/tmp/ansible-local-10715130OGa/ansible-tmp-1572449641.13-1032907249944/AnsiballZ_nxos_lacp.py\", line 40, in invoke_module\n runpy.run_module(mod_name='ansible.modules.network.nxos.nxos_lacp', init_globals=None, run_name='__main__', alter_sys=False)\n File \"/usr/lib/python2.7/runpy.py\", line 192, in run_module\n fname, loader, pkg_name)\n File \"/usr/lib/python2.7/runpy.py\", line 72, in _run_code\n exec code in run_globals\n File \"/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/modules/network/nxos/nxos_lacp.py\", line 188, in <module>\n File \"/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/modules/network/nxos/nxos_lacp.py\", line 183, in main\n File \"/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/module_utils/network/nxos/config/lacp/lacp.py\", line 69, in execute_module\n File \"/tmp/ansible_nxos_lacp_payload_xO14YU/ansible_nxos_lacp_payload.zip/ansible/module_utils/connection.py\", line 187, in __rpc__\nansible.module_utils.connection.ConnectionError: lacp system-mac 00c1.4c00.bd15 role primary\r\r\n ^\r\n% Invalid command at '^' marker.\r\n\rcamden-n9k.example.com(config)# \n",
"module_stdout": "",
"msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
"rc": 1
}
| @@ -31,7 +31,6 @@ | |||
| - name: "Collect interface list" | |||
| nxos_command: | |||
| commands: ['show interface brief | json'] | |||
| timeout: 60 | |||
There was a problem hiding this comment.
This is already addressed by: #63963
| @@ -260,7 +260,10 @@ def dict_diff(base, comparable): | |||
| if not isinstance(base, dict): | |||
| raise AssertionError("`base` must be of type <dict>") | |||
| if not isinstance(comparable, dict): | |||
| raise AssertionError("`comparable` must be of type <dict>") | |||
| if not comparable: | |||
There was a problem hiding this comment.
I think you're testing specifically for None here? if so do:
if comparable is None
There was a problem hiding this comment.
Another question here. Where is this method being called? Should the caller be passing in a <dict> instead of setting comparable to a <dict>?
There was a problem hiding this comment.
dict_diff gets called by many of the nxos/config/* files. I agree that ideally the callers should not pass None to this method, but this method should be protecting itself regardless.
| @@ -133,6 +133,9 @@ def _state_replaced(self, want, have): | |||
| if merged_commands: | |||
| commands.extend(deleted_commands) | |||
| commands.extend(merged_commands) | |||
| else: | |||
| commands.extend(deleted_commands) | |||
|
|
|||
There was a problem hiding this comment.
Nit:
commands.extend(deleted_commands)
if merged_commands:
commands.extend(merged_commands)
There was a problem hiding this comment.
you caught my laziness. will change
| @@ -23,12 +31,25 @@ | |||
| state: deleted | |||
| register: result | |||
|
|
|||
| - debug: | |||
| msg: "{{ result }}" | |||
There was a problem hiding this comment.
yeah, will-do.
|
Shippable terminated your changeset tests because it crossed the 45 minute max runtime. That happened to my PR as well. Everything else was clean though, so when you push your next changeset it should (hopefully) pass. |
| - "'lacp system-priority 11' in result.commands" | ||
| - "'lacp system-mac 00c1.4c00.bd15 role primary' in result.commands" | ||
| - "result.commands|length == 2" | ||
| when: platform is search('N9K') and imagetag is not search('I2|I4|I5|I6') |
There was a problem hiding this comment.
Thanks for adding the imagetag check. Go ahead and add I3 to this list as well. We don't test this in our CI but we should include all tags that are not supported. Also, I believe you can just do this search('I[2-6]') but test it to make sure :)
There was a problem hiding this comment.
Yeah, regex works.
...or make a whitespace change in a comment to trigger a new run. |
|
Closing and reopening the PR to trigger the shippable. |
|
recheck |
ansibot
added
core_review
|
@pabelanger seems like we're hitting the known "Socket is closed" issue with IOS-XR. This is not related to the changes in this PR. Are we okay with merging this? |
ansibot
added
stale_ci
|
@NilashishC yes, iosxr jobs are not voting, so if they are failing, we can consider them not to block merging. You can see Zuul considered the results successful: Build succeeded (third-party-check pipeline). We still want to review the results of iosxr, like you did, to confirm it is iosxr. I plan on adding the newer version of it this coming weeks. |
SUMMARY
Integration test for replace op for nxos_lacp module was failing on some platforms due to not handling the platforms which do not support "lacp system mac" command.
ISSUE TYPE
COMPONENT NAME
nxos_lacp